[terra-popup] Move focus to first interactable child if available. #1476
Conversation
pranav300
requested review from
supreethmr,
lokesh-0813,
saket2403 and
ShettyAkarsh
pranav300
requested review from
benbcai,
ryanthemanuel and
tbiethman
as code owners
ryanthemanuel
temporarily deployed
to
terra-framew-move-focus-quqn7m
ryanthemanuel
temporarily deployed
to
terra-framew-move-focus-quqn7m
|
|
||
| const UpdatedChildren = children.map(child => React.cloneElement(child)); | ||
| UpdatedChildren.every((child, index) => { | ||
| if ((['a', 'button', 'input', 'textarea', 'select', 'details'].includes(child.type) || Object.keys(child.props).includes('tabIndex')) && child.props.tabIndex !== '-1') { |
There was a problem hiding this comment.
Just curious...how did we determine that these are the types to check?
There was a problem hiding this comment.
Only the first layer of children provided to the PopupContent component are being validated here. This logic is not taking their potential interactive content into account. If you were to wrap the inputs in the test component in another div, they would not be detected by this logic.
The 'tabbable' dependency would be helpful in determining which elements are available for receiving focus in this scenario, regardless of those components' depths in the child structure. However, you would have to manually focus the element when the popup opens rather than relying on the autoFocus attribute.
There was a problem hiding this comment.
Updated here ad5c6e0
There was a problem hiding this comment.
If there is an interactable item in the mainContent then that will have focus. otherwise, it will check for interactable items in the headerContent and the first interactable item will be focused there as well. Based on priority the last item to get focus when popup is opened will be the close button
ryanthemanuel
temporarily deployed
to
terra-framew-move-focus-quqn7m
ryanthemanuel
temporarily deployed
to
terra-framew-move-focus-quqn7m
ryanthemanuel
temporarily deployed
to
terra-framew-move-focus-quqn7m
|
The issue is being reassessed. |
Summary
This PR fixes focus issues in the bounded popup where the focus was first received on the close button instead of the first interactable field.
The fix will focus on an interactable field if it is present and is not disabled/hidden. Otherwise, if there is no interactable field it will focus on to the close button.
Part of #1299
Deployment Link
https://terra-framework-deployed-pr-#.herokuapp.com/
Testing
I have added WDIO tests to verify the change.
Additional Details
Thank you for contributing to Terra.
@cerner/terra